feat(pattern): griff-pattern — the std-only structural algebra (S16 Phase 1) by PhysShell · Pull Request #113 · PhysShell/griff

PhysShell · 2026-07-14T17:26:11Z

Summary

The first implementation slice of S16 (ADR-0029): a new workspace member griff-pattern holding the pure structural pattern algebra — Kernel, bounded fractalize, path-addressed pruning (swang-prune-hash-v1), row_major/snake traversals, and ActivitySequence. Everything lowers-to-music lives elsewhere; this crate knows no MIDI, no griff-core, no serde, no floats.

Commit sequence (TDD per AGENTS.md)

Red (540dde9) — 21 failing tests plus the API skeleton, all behavioral bodies unimplemented!() (trivial accessors like NodePath::as_slice were plumbing, not algorithm). The nine swang-prune-hash-v1 golden vectors were computed by an independent BigInteger implementation of spec §1.8 before the crate compiled.
Red fix (e2f7ba6) — the first red's depth arithmetic disagreed with itself (81 cells at depth 2 in one test, depth 1 in another) and mis-addressed a pruned block. With depth 0 = the kernel (spec §1.7), a depth-d grid carries d+1 kernel factors per axis.
Green (ab108bd) — the minimal implementation; all 21 tests pass, including the bit-for-bit match against the independent golden vectors.

What the tests pin

kernel laws: rectangular X/., ragged/invalid/empty are typed errors naming the offending cell (spec §1.6);
expansion laws: active parent → kernel replica, empty parent → entirely empty block, depth 0 = kernel (§1.7);
budgets: required, no library defaults, breach fires before allocation carrying the offending NodePath (§1.4);
pruning: the exact normative algorithm of §1.8 — mix64/DOMAIN/GAMMA fold, constant threshold floor(bps·2^64/10000), edge laws at 0 and 10000 bps, pruned parent → silent 9×9 subtree two levels down, generation-seed independence by construction;
traversals: the spec's §1.9 worked example verbatim (row_major 0 2 3 4 7 8 vs snake 0 2 4 5 7 8 from one kernel), and linearize preserves all 81 cells because silence is a slot (§1.10);
two property tests over random kernels: grid dimensions, cell preservation, the empty-parent law.

Notes for review

cargo clippy --all-targets -- -D warnings is clean; the single arithmetic allow carries its reason.
cargo test --workspace is green except the pre-existing missing_file_golden, which compares the OS strerror text and fails on any non-English Windows locale regardless of branch — Linux CI is unaffected. Worth a follow-up normalization, tracked separately.
thin is deliberately absent: spec §1.10 fixes its type contract but not its selection rule; it gets its own red once the spec section lands (flagged during the design review).

Refs #108, ADR-0029, S16 Phase 1.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

Summary by CodeRabbit

New Features
- Added structural pattern generation from 2D occupancy grids.
- Added configurable expansion depth, cell budgets, and deterministic density-based pruning.
- Added row-major and snake traversal options for converting patterns into activity sequences.
- Added helpers for inspecting active cells and identifying sequence onsets.
- Added validation with clear errors for malformed patterns, invalid density, and exceeded limits.
Tests
- Added comprehensive coverage for pattern parsing, expansion, pruning, traversal, and budget enforcement.

Review round 2 (comment 4972142215)

Blocker 1 (90d6828 red → 69f5b69 green): two-pass kernel validation — the rectangle is judged in a pass that stores nothing, so a ragged-and-foreign row fails as RaggedKernel and nothing allocates for a broken shape. Codex's thread resolved.
Blocker 2 (same red/green): MaxDepthExceeded now carries the NodePath every budget breach owes; the up-front check names the root.
Contract blocker (d823a7a, docs): ADR-0029 §2 and the Phase 1 primitive list now name the real types (Kernel/Expansion/NodePath/PruneSpec) and record that no materialized PatternTree exists — the coordinate digits are the tree (decisions log). thin moves out of Phase 1 and the specified v0.1 roster: its type contract stays in spec §1.10, its selection rule is deliberately unspecified; spec acceptance test 6 now guards the artifact's bar geometry instead.
Minor: prune-hash doc says evaluation-order-independent.

…ash-v1 (S16 Phase 1) Twenty-one failing tests and the API they call, with every body `unimplemented!()`: the tests must appear and fail, not fail to compile. What they pin, from docs/swang/spec.md: - a kernel is rectangular X/. — ragged rows, foreign characters, and emptiness are typed errors naming the offending cell (§1.6); - depth 0 is the kernel itself; an active parent expands into a kernel replica and an empty parent into an entirely empty block (§1.7); - budgets are required and fire *before* allocation, carrying the offending NodePath — 81 cells against a budget of 80 names the root (§1.4); - swang-prune-hash-v1 matches nine golden vectors computed by an independent BigInteger implementation of §1.8 before this crate existed, and the threshold is exactly floor(bps·2^64/10000) — 8000 bps is 14757395258967641292, 5000 bps is 2^63, 0 keeps nothing below the root, 10000 skips the test entirely; - at seed 17 and 5000 bps, child 0 (0xec2c… ≥ 2^63) prunes and its whole subtree stays silent two levels down, while child 2 (0x3a29… < 2^63) survives — the pruned-parent law made concrete; - row_major and snake reproduce the spec's §1.9 worked example (onsets 0 2 3 4 7 8 versus 0 2 4 5 7 8 from one kernel), and linearize preserves all 81 cells because silence is a slot, not an absence (§1.10); - two property tests hold the grid dimensions, the cell-preservation law, and the empty-parent law over random small kernels. The crate is std-only by contract (ADR-0029 §2): its Cargo.toml has an empty [dependencies] section on purpose. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

…itself The budget test claimed depth 2 needs 81 cells while the linearize test put 81 cells at depth 1, and the pruned-parent test addressed child 2's block at a column inside child 0's. With depth 0 defined as the kernel itself (spec §1.7), a depth-d grid carries d + 1 kernel factors per axis: depth 1 is 9×9 = 81 cells, depth 2 is 27×27. The budget test now breaches at depth 1, the property test asserts pow(depth + 1), and the prune test addresses child 2's block where it actually is — column 6 — asserting the full 9×9 silent subtree at depth 2. Still red: bodies remain unimplemented!(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

…6 Phase 1) The minimal implementation behind the red suite: - Kernel::from_rows walks characters once, naming the offending cell before anything allocates; - fractalize computes the whole-grid cost in u128 up front — depth 0 is the kernel, a depth-d grid carries d + 1 kernel factors — and answers each cell from its coordinate digits: most-significant digit first, each digit an active-kernel check, each proper prefix a swang-prune-hash-v1 test against the constant threshold floor(bps·2^64/10000); - the hash is mix64 (Stafford Mix13) folded from mix64(DOMAIN ^ seed), one child index at a time — it reproduces, bit for bit, the nine golden vectors computed by the independent BigInteger implementation before the crate compiled; - linearize reads rows straight or boustrophedon and keeps every cell, because silence is a slot; - no unwrap, no indexing, no floats, no usize in hashed state; the one arithmetic allow carries its reason (strides and dimensions are non-zero by construction). cargo test --workspace is green everywhere except the pre-existing missing_file_golden, which compares the OS's strerror text and fails on any non-English Windows locale regardless of branch; CI's Linux runner is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

coderabbitai · 2026-07-14T17:26:28Z

📝 Walkthrough

Walkthrough

Adds a workspace-integrated griff-pattern crate implementing kernel parsing, bounded fractal expansion, deterministic pruning, traversal linearization, structured errors, and comprehensive unit and property tests.

Changes

Pattern crate

Layer / File(s)	Summary
Crate setup and regression fixtures `Cargo.toml`, `pattern/Cargo.toml`, `pattern/proptest-regressions/lib.txt`	Registers the new workspace member, defines the std-only crate manifest and workspace linting, and adds saved proptest regression seeds.
Pattern data model and traversal `pattern/src/lib.rs`	Defines `Kernel`, expansion configuration and data types, `ActivitySequence`, traversal behavior, and structured error handling.
Expansion, pruning, and validation `pattern/src/lib.rs`	Implements budget-checked expansion, path-addressed hash pruning, exact density thresholds, and tests for parsing, expansion, pruning, traversal, hashing, and invariants.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Kernel
  participant fractalize
  participant Expander
  participant linearize
  participant ActivitySequence
  Kernel->>fractalize: kernel, depth, prune spec, budget
  fractalize->>Expander: expand candidate cells
  Expander->>Expander: evaluate activity and prune paths
  fractalize-->>linearize: Expansion
  linearize->>ActivitySequence: traversal-ordered cells

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding the std-only griff-pattern crate for S16 Phase 1.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/s16-pattern-core

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab108bdd23

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

PhysShell · 2026-07-14T17:33:48Z

Architecture/code review — changes required before merge

The hard part is sound: I independently compared the coordinate-digit evaluator with a recursive substitution model over small rectangular kernels, depths 0–2, and several seed/density combinations. The resulting grids match, and the hash/threshold implementation agrees with the normative §1.8 vectors. The red → red-fix → green sequence is also materially honest.

There are two code blockers and one phase-contract blocker.

1. Validate the complete kernel before allocating

Codex has already opened the correct inline thread. Kernel::from_rows calls Vec::with_capacity before validating later row widths, then pushes cells before discovering a ragged row. This violates the explicit S16 law that ragged kernels are rejected before allocation and can change a typed error into an allocation failure.

Please use a two-pass implementation:

validate non-empty shape, all row lengths, and allowed characters without building cells;
only after successful validation, allocate and populate.

Add a red test pinning shape-validation precedence for an input that is both ragged and contains a foreign character on the ragged row. The result should be RaggedKernel, proving the shape pass completes before cell decoding. Then land the green implementation separately.

2. Every structural budget breach must carry `NodePath`

PatternError::MaxCellsExceeded carries a path, but MaxDepthExceeded does not. The normative spec and Phase-1 acceptance say a budget breach carries the offending NodePath, covering both max_depth and max_cells.

Add a red test requiring:

PatternError::MaxDepthExceeded {
    path: NodePath::default(),
    depth: 3,
    max_depth: 2,
}

Then add the field in a separate green commit. The root is the correct offending path for the up-front global depth check.

3. Do not declare Phase 1 closed or start Phase 2 with the current contract drift

The implementation is a good Phase-1 core slice, but the accepted stage contract currently also requires thin, and the documented primitive roster names Pattern, PatternTree, and FractalSpec. This PR intentionally ships none of those exact contracts: it uses Kernel, a flattened Expansion, and direct depth/prune/budget arguments. That may be the better API, but documents and implementation cannot quietly diverge.

Before moving to Phase 2, choose one honest path:

define the missing thin selection semantics and land it red→green, while explaining how Expansion satisfies or replaces PatternTree/FractalSpec; or
amend the still-Proposed ADR/S16/spec contract now: defer thin until its selector is specified, and replace the planned type roster with the actual v0.1 representation (Kernel, Expansion, NodePath, PruneSpec, ExpansionBudget). Record why the direct coordinate evaluator avoids a materialized tree.

I prefer the second option. Inventing thin merely to satisfy a stale bullet would be governance by checkbox, one of humanity’s more persistent recreational errors.

Minor cleanup

Change prune_hash_v1’s documentation from “order-independent” to “evaluation-order-independent”; path order is intentionally significant.

CI is green on ab108bdd23d03dc23c6e4b326c6cba6bdf00e0ce. After the two red/green fixes and the contract alignment, this is mergeable. The pre-existing localized missing_file_golden is unrelated and not a blocker here.

coderabbitai

🧹 Nitpick comments (1)

pattern/src/lib.rs (1)
109-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate is_active/active_count logic between Kernel and Expansion.

Kernel::is_active/Kernel::active_count (Lines 109-125) and Expansion::is_active/Expansion::active_count (Lines 219-235) are byte-for-byte identical implementations operating on the same {width, cells: Vec<bool>} shape. Consider extracting a small shared internal grid type (or a private trait with default methods given width()/cells() accessors) that both Kernel and Expansion delegate to, so the bounds-check logic (including the important col >= width guard that prevents flat-index wraparound into the next row) only exists once and can't silently drift between the two copies in a future edit.
♻️ Sketch of a shared grid helper
struct Grid {
    width: usize,
    height: usize,
    cells: Vec<bool>,
}

impl Grid {
    fn is_active(&self, row: usize, col: usize) -> bool {
        if col >= self.width {
            return false;
        }
        cell_index(self.width, row, col)
            .and_then(|index| self.cells.get(index))
            .copied()
            .unwrap_or(false)
    }

    fn active_count(&self) -> usize {
        self.cells.iter().filter(|&&cell| cell).count()
    }
}
Kernel and Expansion can then hold a Grid field (or newtype-wrap it) and forward their public is_active/active_count to it.
Also applies to: 219-235
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pattern/src/lib.rs` around lines 109 - 125, Extract the duplicated grid
behavior from Kernel and Expansion into one private shared grid abstraction,
such as Grid, containing the width and cells data and implementing is_active and
active_count. Update both types’ public methods to delegate to this shared
implementation while preserving the col >= width guard and existing out-of-range
behavior; avoid maintaining separate copies of the bounds-check and counting
logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pattern/src/lib.rs`:
- Around line 109-125: Extract the duplicated grid behavior from Kernel and
Expansion into one private shared grid abstraction, such as Grid, containing the
width and cells data and implementing is_active and active_count. Update both
types’ public methods to delegate to this shared implementation while preserving
the col >= width guard and existing out-of-range behavior; avoid maintaining
separate copies of the bounds-check and counting logic.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b3e7ed5-4362-4931-8e8d-a9e28659d166

📥 Commits

Reviewing files that changed from the base of the PR and between 8e7c2ac and ab108bd.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

Cargo.toml
pattern/Cargo.toml
pattern/proptest-regressions/lib.txt
pattern/src/lib.rs

…ts path Two review findings from #113, pinned before they harden into historically-grown behavior: - a row that is both ragged and carries a foreign character must fail as RaggedKernel, because the spec validates the rectangle before any cell decodes (and before anything allocates) — today the character wins, which proves decoding runs first; - MaxDepthExceeded gains the NodePath every budget breach owes by contract; the up-front whole-expansion check names the root. The variant carries the field now, the construction site says unimplemented!() until green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

… carries a path The rectangle is judged in a first pass that stores nothing, so a row that is both ragged and foreign fails as RaggedKernel and no Vec exists for a kernel whose shape is already broken; cells decode in a second pass. MaxDepthExceeded fills the NodePath it now owes — the root, for the up-front whole-expansion check — and its Display says so. The prune-hash doc also stops overclaiming: it is evaluation-order- independent, not order-independent; the path's own element order is load-bearing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

…he tree implicit The stage plan promised Pattern/PatternTree/FractalSpec; the crate that passed review ships Kernel/Expansion/NodePath/PruneSpec — and no materialized tree at all, because the coordinate digits are the tree. Recognize that in ADR-0029 §2, the Phase 1 primitive list, and the decisions log, so the next agent extends the addressing scheme instead of summoning a second type family. thin moves out of Phase 1 and out of the specified v0.1 roster: its type contract stays fixed in spec §1.10, its selection rule is deliberately unspecified, and acceptance test 6 now guards the artifact's bar geometry instead of an operator that ships in no phase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

PhysShell and others added 3 commits July 14, 2026 22:12

chatgpt-codex-connector Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread pattern/src/lib.rs

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

PhysShell and others added 3 commits July 14, 2026 22:39

PhysShell merged commit 0910db2 into main Jul 14, 2026
4 checks passed

This was referenced Jul 14, 2026

feat(swang): griff-swang — the sequence meets the bar (S16 Phase 2, slice 1) #114

Merged

feat(cli): --rhythm-* — the transport syntax reaches the generator (S16 Phase 2, slice 3) #116

Merged

This was referenced Jul 15, 2026

S16 Phase 3, slice 1: the Swang parser, canonical formatter, and their CLI edge #118

Merged

S8 Swang Playground, Slice 1: text → diagnostics → candidates → piano-roll → export #123

Merged

Navigation Menu

feat(pattern): griff-pattern — the std-only structural algebra (S16 Phase 1) - #113

feat(pattern): griff-pattern — the std-only structural algebra (S16 Phase 1)#113
PhysShell merged 6 commits into
mainfrom
claude/s16-pattern-core

PhysShell commented Jul 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

PhysShell commented Jul 14, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

PhysShell commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commit sequence (TDD per AGENTS.md)

What the tests pin

Notes for review

Summary by CodeRabbit

Review round 2 (comment 4972142215)

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

PhysShell commented Jul 14, 2026

Architecture/code review — changes required before merge

1. Validate the complete kernel before allocating

2. Every structural budget breach must carry NodePath

3. Do not declare Phase 1 closed or start Phase 2 with the current contract drift

Minor cleanup

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

PhysShell commented Jul 14, 2026 •

edited

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

2. Every structural budget breach must carry `NodePath`